Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: Enable properties file in flank scripts #1450

Merged
merged 19 commits into from
Jan 11, 2021
Merged

refactor: Enable properties file in flank scripts #1450

merged 19 commits into from
Jan 11, 2021

Conversation

pawelpasterz
Copy link
Contributor

@pawelpasterz pawelpasterz commented Jan 4, 2021 •
edited
Loading

Enable properties file to be used when testing flank-scripts.
Now you can use flank-debug.properties to replace some crucial settings (like repo). Custom properties are used only during local runtime (CI and tests use the defaults)

Test plan

  1. Build project ./gradlew build
  2. Observe all tests finish without an error
  3. /common/flank-debug.properties should be created
  4. Run java -jar ./flank-scripts/build/libs/flank-scripts.jar shell firebase checkForSdkUpdate --github-token=[your gh token] --zenhub-token=""
  5. Default settings are used, there should be similar out to: 
    ** Last workflow run:
 name: Update dependencies
 last run: 2021-01-06T10:05:04Z
 url: https://github.com/Flank/flank/actions/runs/465789230
  6. In flank-debug.properties uncomment repo.flank and change to any random value
  7. Run again java -jar...
  8. Output should contain ** No workflow run found for update_dependencies_and_client.yml
  9. Comment/undo repo.flank and change value for sdk-check.workflow-filename
  10. Run java -jar....
  11. There should be output with ** No workflow run found for [name you changed]

Please, feel free to play with other scritps

@pawelpasterz pawelpasterz self-assigned this Jan 4, 2021
@pawelpasterz pawelpasterz force-pushed the enable-properties-file-in-flank-scripts branch from 69a8f44 to 279f041 Compare January 4, 2021 15:46
@piotradamczyk5
Copy link
Contributor

I think that this properties logic should be repository-wide (usable for flank, flank-scripts, integration tests etc).
We should have also property for Github token, HEAD_REF etc

@pawelpasterz
Copy link
Contributor Author

pawelpasterz commented Jan 6, 2021 •
edited
Loading

I think that this properties logic should be repository-wide (usable for flank, flank-scripts, integration tests etc).

I really like the idea! This PR has already 30+ files changed. Let's make it in separate PR (PRs), please 🙏

@pawelpasterz pawelpasterz force-pushed the enable-properties-file-in-flank-scripts branch from ee996a9 to 97e57ff Compare January 6, 2021 06:56
@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2021 •
edited
Loading

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2021 •
edited
Loading

Timestamp: 2021-01-11 07:04:44
Buildscan url for ubuntu-workflow run 476923850
https://gradle.com/s/rgpyxrtrhvczw

@pawelpasterz pawelpasterz force-pushed the enable-properties-file-in-flank-scripts branch from b4adf9e to 74b0399 Compare January 6, 2021 07:56
@piotradamczyk5
Copy link
Contributor

@Mergifyio rebase

@mergify
Copy link

mergify bot commented Jan 7, 2021

Command rebase: success

Branch has been successfully rebased

@bootstraponline bootstraponline force-pushed the enable-properties-file-in-flank-scripts branch from 4258850 to b61dbd8 Compare January 7, 2021 13:13
@pawelpasterz
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

github-actions bot added a commit that referenced this pull request Jan 7, 2021
@github-actions
@pawelpasterz has signed the CLA from Pull Request #1450
61585d1
@pawelpasterz
Copy link
Contributor Author

@flank-it

@github-actions
Copy link
Contributor

github-actions bot commented Jan 7, 2021 •
edited
Loading

Integration tests succeed ✅
Build scan https://gradle.com/s/76qqpkvaasxsg
Workflow run https://github.com/Flank/flank/actions/runs/469056770

@pawelpasterz pawelpasterz marked this pull request as ready for review January 7, 2021 14:15
@jan-goral
Copy link
Contributor

jan-goral commented Jan 8, 2021 •
edited
Loading

This PR is a feature, not a refactor because it is adding new options to flank-scripts. The refactor is a type of change that is not changing any external behaviour of the application. Will be good to have an issue linked to this PR with the described needs.

@pawelpasterz
Copy link
Contributor Author

IMO it's a feature, not a refactor. Will be good to have an issue linked to this PR with the described needs.

Good point, I'll create one

@piotradamczyk5 piotradamczyk5 self-requested a review January 8, 2021 17:11
Sloox
Sloox previously requested changes Jan 9, 2021
View reviewed changes
Copy link
Contributor

@Sloox Sloox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has it been tested on Windows?

@pawelpasterz
Copy link
Contributor Author

Has it been tested on Windows?

It hasn't. I don't have windows machine unfortunately :(

adamfilipow92
adamfilipow92 approved these changes Jan 11, 2021
View reviewed changes
Copy link
Contributor

@adamfilipow92 adamfilipow92 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Windows

  • Build and run command works. Here is my output
ken=""
** Find previously checked commit
** Last workflow run:
     name: Update dependencies
     last run: 2021-01-06T10:05:04Z
     url: https://github.com/Flank/flank/actions/runs/465789230
** Commit found:
      SHA: 8de5b70bc8996687e097d604815018c7b60f29d3
      timestamp: 2020-12-16T04:09:10Z
** Find latest commit
** Commit found:
      SHA: 8de5b70bc8996687e097d604815018c7b60f29d3
      timestamp: 2020-12-16T04:09:10Z
** No new commits since the last run
  • properties saved
  • second run contains ** No workflow run found for update_dependencies_and_client.yml
  • Changing value of sdk-check.workflow-filename works and output contains** No workflow run found for update_dependencies_and_client.yml__

@pawelpasterz pawelpasterz force-pushed the enable-properties-file-in-flank-scripts branch from 35eba64 to d55f525 Compare January 11, 2021 06:35
@bootstraponline bootstraponline force-pushed the enable-properties-file-in-flank-scripts branch from d55f525 to 0f53b52 Compare January 11, 2021 06:35
@pawelpasterz pawelpasterz dismissed Sloox’s stale review January 11, 2021 06:46

Tested on windows

@pawelpasterz pawelpasterz force-pushed the enable-properties-file-in-flank-scripts branch from 8645d71 to 1fcde06 Compare January 11, 2021 06:55
@mergify mergify bot merged commit f4916e5 into master Jan 11, 2021
@mergify mergify bot deleted the enable-properties-file-in-flank-scripts branch January 11, 2021 07:05
@github-actions github-actions bot locked and limited conversation to collaborators Jan 11, 2021
@zuziaka zuziaka added this to the Sprint -1 milestone Jan 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@piotradamczyk5 piotradamczyk5 piotradamczyk5 approved these changes

@adamfilipow92 adamfilipow92 adamfilipow92 approved these changes

@Sloox Sloox Sloox left review comments

@jan-goral jan-goral Awaiting requested review from jan-goral

@zuziaka zuziaka Awaiting requested review from zuziaka

Assignees

@pawelpasterz pawelpasterz

@piotradamczyk5 piotradamczyk5

Labels
Tiger 🐯
Projects
None yet
Milestone
Sprint -1
Development

Successfully merging this pull request may close these issues.
6 participants
@pawelpasterz @piotradamczyk5 @jan-goral @Sloox @adamfilipow92 @zuziaka